Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Update BundleUpload status for observability" #112

Merged
merged 1 commit into from
Oct 3, 2024
Merged

Conversation

gnalh
Copy link
Collaborator

@gnalh gnalh commented Oct 3, 2024

Reverts #99

I've narrowed down a breakage that I've been seeing locally to this PR. It is the first time uploads start to fail in the check and it makes changes to the area of code in question.
https://github.com/trunk-io/analytics-cli/actions/runs/11113171931/job/30876881217#step:9:26

It wasn't caught because we're returning exit code 0 even when the upload fails. So the job looked like it passed. We need to add a mode that forces fail on any error so we can better test the cli.

I'm putting up a revert in case the actual fix is non trivial. Feel free to close this if we can get a proper fix in in a timely manner.

After revert everything looks as expected:

2024-10-02T20:44:39 [INFO] - Reading git repo at "/Users/work/src/iOS-example"
2024-10-02T20:44:39 [INFO] - Found git_url: Some("git@github.com:gnalh/iOS-example.git")
2024-10-02T20:44:39 [INFO] - Found git_sha: Some("3959a8720720b3ce83309ecdb34086ebb167afcc")
2024-10-02T20:44:39 [INFO] - Found git_branch: Some("refs/heads/main")
2024-10-02T20:44:39 [INFO] - Found git_commit_time: Time { seconds: 1727915273, offset: -25200, sign: Minus }
2024-10-02T20:44:39 [INFO] - Found git_commit_message: Some("test\n")
2024-10-02T20:44:39 [INFO] - Found git_author: Some(HeadAuthor { name: "gnalh", email: "gabe@trunk.io" })
2024-10-02T20:44:39 [INFO] - Starting trunk-analytics-cli 0.0.0 (git=e398c742381c564e9118f6a49488c37e1eb51c13) rustc=1.80.0-nightly
2024-10-02T20:44:40 [INFO] - Total files pack and upload: 1
2024-10-02T20:44:40 [INFO] - Total bytes in: 77753, total bytes out: 15196 (size reduction: 80.46%)
2024-10-02T20:44:40 [INFO] - Flushed temporary tarball to "/var/folders/3y/n_0cx1_94sx4h8xq510svyh00000gn/T/.tmplqjRIF/bundle.tar.zstd"
2024-10-02T20:44:40 [INFO] - Done

Copy link

trunk-staging-io bot commented Oct 3, 2024

✅ 86 passed ⋅ (learn more)

settingsdocs ⋅ learn more about trunk.io

@dfrankland
Copy link
Member

We can definitely add tests that assert the test should fail—testing both happy and sad paths:

// should fail due to quarantine and succeed without quarantining
.failure();

@dfrankland
Copy link
Member

Also, my PR #109 conflicts with #99, so I want to know whether or not we revert 😅

Copy link
Contributor

@max-trunk max-trunk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry about this! Was able to reproduce the error log when invoking an upload against staging

max@max-cloudtop:~/src/flake-factory$ TRUNK_PUBLIC_API_ADDRESS=https://api.trunk-staging.io:443 ~/src/analytics-cli/target/x86_64-unknown-linux-musl/release/trunk-analytics-cli upload --org-url-slug trunk-staging-org --token 8ee28021f4e9168f075c20ef732fbb475599c5d0 --junit-paths "**/gotestsum_test.xml"
2024-10-03T12:29:01 [INFO] - Reading git repo at "/home/max/src/flake-factory"
2024-10-03T12:29:01 [INFO] - Found git_url: Some("git@github.com:trunk-io/flake-factory.git")
2024-10-03T12:29:01 [INFO] - Found git_sha: Some("e9665845818c1902ada378ff620dcc5101d1a1bf")
2024-10-03T12:29:01 [INFO] - Found git_branch: Some("refs/heads/main")
2024-10-03T12:29:01 [INFO] - Found git_commit_time: Time { seconds: 1727892546, offset: -25200, sign: Minus }
2024-10-03T12:29:01 [INFO] - Found git_commit_message: Some("Merge pull request #8393 from trunk-io/add-karma-framework")
2024-10-03T12:29:01 [INFO] - Found git_author: Some(HeadAuthor { name: "Josh Marinacci", email: "276938+joshmarinacci@users.noreply.github.com" })
2024-10-03T12:29:01 [INFO] - Starting trunk-analytics-cli 0.0.0 (git=f81ac924d5687b4cd1115709fa673ba06088d9a6) rustc=1.80.0-nightly
2024-10-03T12:29:02 [INFO] - Total files pack and upload: 1
2024-10-03T12:29:02 [INFO] - Total bytes in: 4568, total bytes out: 2154 (size reduction: 52.85%)
2024-10-03T12:29:02 [INFO] - Flushed temporary tarball to "/tmp/.tmp6LV7bk/bundle.tar.zstd"
2024-10-03T12:29:22 [WARN] - Failed to update bundle upload status to UploadComplete: Failed to update bundle upload status
2024-10-03T12:29:22 [INFO] - Done

But I do see the resulting row with its status correctly updated in the metrics db

Screenshot 2024-10-03 at 8 31 54 AM

But also, the CLI invocation was much slower - is it possible there's some timeout getting triggered in one of the services?

I think it makes sense to revert for now while we sort this out. I could see us needing to add / revise an index on bundle_upload

@gnalh gnalh merged commit 78f867d into main Oct 3, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants